[WIP] feat(pdp): support create and upload #692

Kubuxu · 2025-10-09T15:32:43Z

No description provided.

Kubuxu · 2025-10-09T15:56:13Z

tasks/pdp/proofset_addroot_watch.go

+			return fmt.Errorf("expeted to find dataSetId in receipt but failed to extract: %w", err)
+		}
+		// XXX: I considered here chekcing if dataset exists already in DB, but not sure if it is needed
+	}


The majority of my questions are around this file. I'm yet to test it, but I'm using dataset=0 as a sentinel value.

It might be better if it were maybe NULL. Also, I don't know about any possible table relations that this might affect.
@LexLuthr

I think you should look at handleGetPieceAdditionStatus, that's the only place I can see where it might matter - client can ask for piece addition status and it selects by data set id; what should the client ask for in the case of the combined flow and what do we expect them to get, because I think maybe none of it works currently. We call that with getPieceAdditionStatus in the SDK.

The only other place where we have functionality outside of this PR that impacts that table is the trigger we add for transaction resolving and it doesn't care about data set id:

curio/harmony/harmonydb/sql/20250730-pdp-rename.sql

Lines 155 to 170 in 435f7b8

CREATE OR REPLACE FUNCTION update_pdp_data_set_piece_adds()

RETURNS TRIGGER AS $$

BEGIN

IF OLD.tx_status = 'pending' AND (NEW.tx_status = 'confirmed' OR NEW.tx_status = 'failed') THEN

-- Update the add_message_ok field in pdp_data_set_piece_adds if a matching entry exists

UPDATE pdp_data_set_piece_adds

SET add_message_ok = CASE

WHEN NEW.tx_status = 'failed' OR NEW.tx_success = FALSE THEN FALSE

WHEN NEW.tx_status = 'confirmed' AND NEW.tx_success = TRUE THEN TRUE

ELSE add_message_ok

END

WHERE add_message_hash = NEW.signed_tx_hash;

END IF;

RETURN NEW;

END;

$$ LANGUAGE plpgsql;

I could special-case dataset=0 in handleGetPieceAdditionStatus

I think the client should ask for the dataset creation status instead. After that, and after the piece gets processed, the get status will work because the pdp_data_set_piece_adds.data_set will get updated to the proper dataset.

Kubuxu · 2025-10-10T10:32:49Z

@rvagg would appriciate you taking a look as well.

pdp/handlers.go

rvagg · 2025-10-10T11:40:33Z

pdp/handlers_create.go

+
+	type RequestBody struct {
+		RecordKeeper string            `json:"recordKeeper"`
+		Pieces       []AddPieceRequest `json:"pieces"`


oh yeah, nicely typed

pdp/handlers_create.go

tasks/pdp/proofset_addroot_watch.go

rvagg · 2025-10-10T12:06:36Z

Looks good, my only concern is the impact on handleGetPieceAdditionStatus

Kubuxu · 2025-10-14T14:16:07Z

Looking at schema of pdp_data_set_piece_adds. The insert will fail due to foreign key pdp_proofset_root_adds_proofset_fkey as the pdp_data_sets.id won't exist at the time.

[email protected]:yugabyte> \d pdp_data_set_piece_adds
+-------------------+---------+-------------------------+
| Column            | Type    | Modifiers               |
|-------------------+---------+-------------------------|
| data_set          | bigint  |  not null               |
| piece             | text    |  not null               |
| add_message_hash  | text    |  not null               |
| add_message_ok    | boolean |                         |
| add_message_index | bigint  |  not null               |
| sub_piece         | text    |  not null               |
| sub_piece_offset  | bigint  |  not null               |
| sub_piece_size    | bigint  |  not null               |
| pdp_pieceref      | bigint  |  not null               |
| pieces_added      | boolean |  not null default false |
+-------------------+---------+-------------------------+
Indexes:
    "pdp_data_set_piece_adds_pk" PRIMARY KEY, lsm (data_set HASH, add_message_hash ASC, add_message_index ASC)
    "idx_pdp_data_set_piece_adds_pieces_added" lsm (pieces_added HASH)
Foreign-key constraints:
    "pdp_proofset_root_adds_add_message_hash_fkey" FOREIGN KEY (add_message_hash) REFERENCES message_waits_eth(signed_tx_hash) ON DELETE CASCADE
    "pdp_proofset_root_adds_pdp_pieceref_fkey" FOREIGN KEY (pdp_pieceref) REFERENCES pdp_piecerefs(id) ON DELETE SET NULL
    "pdp_proofset_root_adds_proofset_fkey" FOREIGN KEY (data_set) REFERENCES pdp_data_sets(id) ON DELETE CASCADE

I welcome suggestions on how to solve it cleanly, but it seems like transitioning to dataset being NULLable is the cleanest.

Kubuxu · 2025-10-14T15:01:43Z

I pushed a commit with a nullable dataset id in pdp_data_set_piece_adds, otherwise, the foreign key constraint would get in the way.

Kubuxu · 2025-10-14T15:09:40Z

Ohh, data_set cannot be nullable because it is a part of the primary key
Should we just remove it from the PK? I don't think we support flows where a single message adds to multiple different datasets, so that uniqueness constraint is not needed.

LexLuthr

Looking at the table, I don't see any way to get around the problem while keeping the primary key.

LexLuthr · 2025-10-15T14:37:58Z

tasks/pdp/proofset_addroot_watch.go

+	resolvedDataSetId := pieceAdd.DataSet
+	if !resolvedDataSetId.Valid {
+		var err error
+		resolvedDataSetId.Int64, err = extractDataSetIdFromReceipt(receipt)
+		if err != nil {
+			return fmt.Errorf("expeted to find dataSetId in receipt but failed to extract: %w", err)
+		}
+		resolvedDataSetId.Valid = true
+		var exists bool
+		// we check if the dataset exists already to avoid foreign key violation
+		err = db.QueryRow(ctx, `
+			SELECT EXISTS (
+				SELECT 1
+				FROM pdp_data_sets
+				WHERE id = $1
+			)`, resolvedDataSetId.Int64).Scan(&exists)
+		if err != nil {
+			return fmt.Errorf("failed to check if data set exists: %w", err)
+		}
+		if !exists {
+			// XXX: maybe return nil instead to avoid warning?
+			return fmt.Errorf("data set %d not found in pdp_data_sets", resolvedDataSetId.Int64)
+		}
+	}


I would prefer not try to create_watch here as well. Just scan from DB and if it is NULL then just skip processing addPiece for this tipset. But that might cause unnecessary delay of 1 tipset. Any other ideas do this this without 2 watches?

Just scan from DB and if it is NULL then just skip processing addPiece for this tipset

That is what I'm doing here. If we get an add_piece triggered, and the dataset doesn't exist yet, we leave it alone and wait for another trigger for when the dataset is created.
A cleaner way might be to get create_watch to be processed with a higher priority than add_pieces. This could be achieved by combining them into one watcher and executing sequentially with create first.

After processing it sequentially through the combined watcher, I think we could obtain the dataset ID from the pdp_data_sets based on the create_message_hash.

Signed-off-by: Jakub Sztandera <[email protected]>

Kubuxu requested a review from a team as a code owner October 9, 2025 15:32

Kubuxu force-pushed the pdp/create-and-upload branch from 64afc4a to 726cf4d Compare October 9, 2025 15:53

Kubuxu commented Oct 9, 2025

View reviewed changes

Kubuxu changed the title ~~feat(pdp): support create and upload~~ [WIP] feat(pdp): support create and upload Oct 9, 2025

rjan90 mentioned this pull request Oct 10, 2025

chore: fix breaking changes due to PDP v2.2.0 #685

Closed

rvagg reviewed Oct 10, 2025

View reviewed changes

pdp/handlers.go Outdated Show resolved Hide resolved

rvagg reviewed Oct 10, 2025

View reviewed changes

pdp/handlers_create.go Outdated Show resolved Hide resolved

rvagg reviewed Oct 10, 2025

View reviewed changes

pdp/handlers_create.go Outdated Show resolved Hide resolved

rvagg reviewed Oct 10, 2025

View reviewed changes

tasks/pdp/proofset_addroot_watch.go Show resolved Hide resolved

rjan90 linked an issue Oct 13, 2025 that may be closed by this pull request

PDPv0: uploadAndCreate #678

Open

Kubuxu force-pushed the pdp/create-and-upload branch 3 times, most recently from 4ac5aa6 to 3224e22 Compare October 14, 2025 13:58

Kubuxu mentioned this pull request Oct 14, 2025

pdp(chore): extract upload and create handlers into separate files #709

Merged

Kubuxu requested a review from rvagg October 15, 2025 10:45

LexLuthr approved these changes Oct 15, 2025

View reviewed changes

Kubuxu added 6 commits October 15, 2025 17:28

feat(pdp): support create and upload

986039f

Signed-off-by: Jakub Sztandera <[email protected]>

address lint

357f1ff

Signed-off-by: Jakub Sztandera <[email protected]>

lint part 2

56ae7ed

Signed-off-by: Jakub Sztandera <[email protected]>

use nullable data_set id in pdp_data_set_piece_adds to bypass FK

313fca6

Signed-off-by: Jakub Sztandera <[email protected]>

remove data_set from pdp_data_set_piece_adds primary key

46400c0

Signed-off-by: Jakub Sztandera <[email protected]>

fix build issues

3431b2d

Signed-off-by: Jakub Sztandera <[email protected]>

Kubuxu force-pushed the pdp/create-and-upload branch from a17f27f to 44a4e47 Compare October 15, 2025 15:28

combine pdp watchers for optimistic sequence create and add

c18dbf2

Signed-off-by: Jakub Sztandera <[email protected]>

Kubuxu force-pushed the pdp/create-and-upload branch from 44a4e47 to c18dbf2 Compare October 15, 2025 15:28

	CREATE OR REPLACE FUNCTION update_pdp_data_set_piece_adds()
	RETURNS TRIGGER AS $$
	BEGIN
	IF OLD.tx_status = 'pending' AND (NEW.tx_status = 'confirmed' OR NEW.tx_status = 'failed') THEN
	-- Update the add_message_ok field in pdp_data_set_piece_adds if a matching entry exists
	UPDATE pdp_data_set_piece_adds
	SET add_message_ok = CASE
	WHEN NEW.tx_status = 'failed' OR NEW.tx_success = FALSE THEN FALSE
	WHEN NEW.tx_status = 'confirmed' AND NEW.tx_success = TRUE THEN TRUE
	ELSE add_message_ok
	END
	WHERE add_message_hash = NEW.signed_tx_hash;
	END IF;
	RETURN NEW;
	END;
	$$ LANGUAGE plpgsql;

[WIP] feat(pdp): support create and upload #692

Are you sure you want to change the base?

[WIP] feat(pdp): support create and upload #692

Conversation

Kubuxu commented Oct 9, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Kubuxu commented Oct 10, 2025

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

rvagg commented Oct 10, 2025

Uh oh!

Kubuxu commented Oct 14, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Kubuxu commented Oct 14, 2025

Uh oh!

Kubuxu commented Oct 14, 2025

Uh oh!

LexLuthr left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Kubuxu commented Oct 14, 2025 •

edited

Loading